Add explicit broker workspace join options#1055
Conversation
📝 WalkthroughWalkthroughThe PR adds explicit workspace-key and instance-name support across the broker and harness-driver, enabling them to join existing Relay workspaces with stable identifiers. CLI flags, environment-variable resolution, auth registration logic, env propagation, and harness-driver spawn configuration are all updated to prefer canonical variables while preserving legacy fallbacks. ChangesWorkspace-key and Instance-name Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for explicit workspace keys and broker instance names across the broker and harness driver, allowing local and cloud brokers to join the same Relay workspace with stable, addressable names. It adds new CLI flags and environment variables, updates the authentication client to handle explicit joins, and updates the harness driver to support these options. A critical issue was identified in crates/broker/src/runtime/init.rs where std::env::set_var is called without an unsafe block, which will cause a compilation error in Rust 1.79+.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if let Some(workspace_key) = cmd.resolved_workspace_key() { | ||
| std::env::set_var("AGENT_RELAY_WORKSPACE_KEY", &workspace_key); | ||
| std::env::set_var("RELAY_WORKSPACE_KEY", &workspace_key); | ||
| std::env::set_var("RELAY_API_KEY", &workspace_key); | ||
| } |
There was a problem hiding this comment.
In Rust 1.79+, std::env::set_var is an unsafe function. Calling it without an unsafe block will cause a compilation error (error[E0133]: call to unsafe function is unsafe and requires unsafe function or block). Since this codebase uses a Rust version where set_var is unsafe (as seen in the test suite), these calls must be wrapped in an unsafe block.
if let Some(workspace_key) = cmd.resolved_workspace_key() {
unsafe {
std::env::set_var("AGENT_RELAY_WORKSPACE_KEY", &workspace_key);
std::env::set_var("RELAY_WORKSPACE_KEY", &workspace_key);
std::env::set_var("RELAY_API_KEY", &workspace_key);
}
}
|
Consumer-side review (cloud-lead, T4 lane) — no blockers; cloud's env-injection contract is satisfied exactly. Two observations:
Also confirmed from the cloud/pear assumption side: error contexts |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/harness-driver/src/client.ts">
<violation number="1" location="packages/harness-driver/src/client.ts:490">
P2: Workspace key is passed as a CLI argument, which exposes the secret via process args and startup error logging.</violation>
</file>
<file name="crates/broker/src/cli/mod.rs">
<violation number="1" location="crates/broker/src/cli/mod.rs:262">
P2: `resolved_instance_name` lets blank `--instance-name` override fallback/default naming because it trims after precedence resolution.</violation>
<violation number="2" location="crates/broker/src/cli/mod.rs:280">
P2: Blank `--workspace-key` values are treated as present too early, which can suppress env fallback and silently fall back to workspace creation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ...(workspaceKey ? ['--workspace-key', workspaceKey] : []), | ||
| '--channels', |
There was a problem hiding this comment.
P2: Workspace key is passed as a CLI argument, which exposes the secret via process args and startup error logging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/harness-driver/src/client.ts, line 490:
<comment>Workspace key is passed as a CLI argument, which exposes the secret via process args and startup error logging.</comment>
<file context>
@@ -454,9 +473,25 @@ export class HarnessDriverClient {
+ 'init',
+ '--instance-name',
+ brokerName,
+ ...(workspaceKey ? ['--workspace-key', workspaceKey] : []),
+ '--channels',
+ channels.join(','),
</file context>
| ...(workspaceKey ? ['--workspace-key', workspaceKey] : []), | |
| '--channels', | |
| '--channels', |
| self.workspace_key | ||
| .clone() | ||
| .or_else(|| std::env::var("AGENT_RELAY_WORKSPACE_KEY").ok()) | ||
| .map(|key| key.trim().to_string()) | ||
| .filter(|key| !key.is_empty()) |
There was a problem hiding this comment.
P2: Blank --workspace-key values are treated as present too early, which can suppress env fallback and silently fall back to workspace creation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/broker/src/cli/mod.rs, line 280:
<comment>Blank `--workspace-key` values are treated as present too early, which can suppress env fallback and silently fall back to workspace creation.</comment>
<file context>
@@ -248,6 +257,34 @@ pub(crate) struct InitCommand {
+ }
+
+ pub(crate) fn resolved_workspace_key(&self) -> Option<String> {
+ self.workspace_key
+ .clone()
+ .or_else(|| std::env::var("AGENT_RELAY_WORKSPACE_KEY").ok())
</file context>
| self.workspace_key | |
| .clone() | |
| .or_else(|| std::env::var("AGENT_RELAY_WORKSPACE_KEY").ok()) | |
| .map(|key| key.trim().to_string()) | |
| .filter(|key| !key.is_empty()) | |
| self.workspace_key | |
| .as_deref() | |
| .map(str::trim) | |
| .filter(|key| !key.is_empty()) | |
| .map(ToOwned::to_owned) | |
| .or_else(|| { | |
| std::env::var("AGENT_RELAY_WORKSPACE_KEY") | |
| .ok() | |
| .map(|key| key.trim().to_string()) | |
| .filter(|key| !key.is_empty()) | |
| }) |
|
Fixed a broker CLI precedence bug in crates/broker/src/cli/mod.rs: explicit Validated locally:
|
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
✅ pr-reviewer applied fixes — committed and pushed Fixed a broker CLI precedence bug in crates/broker/src/cli/mod.rs: explicit Validated locally:
|
|
Fixed the PR issues I found:
Verification run:
|
|
Fixed the PR issues I found:
Verification run:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/broker/src/cli/mod.rs`:
- Around line 261-277: The resolved_instance_name function currently treats
whitespace-only values (from self.instance_name or AGENT_RELAY_BROKER_NAME) as
present and short-circuits fallback; change the chaining to normalize and filter
out empty/whitespace-only strings before fallback. Specifically, in
resolved_instance_name, map self.instance_name and the env var
(std::env::var("AGENT_RELAY_BROKER_NAME")) to trimmed strings and .filter(|s|
!s.is_empty()) so they become None when blank, then fall back to fallback and
finally unwrap_or_default(), returning the trimmed final result; use the
function name resolved_instance_name to locate the change.
In `@packages/harness-driver/src/client.ts`:
- Around line 456-463: The code is incorrectly treating legacy RELAY_API_KEY as
an explicit --workspace-key by chaining all sources into workspaceKey via
nonEmptyString; split the resolution into two distinct variables (e.g.,
explicitWorkspaceKey = first non-empty of options?.workspaceKey,
options?.env?.AGENT_RELAY_WORKSPACE_KEY, options?.env?.RELAY_WORKSPACE_KEY,
process.env.AGENT_RELAY_WORKSPACE_KEY, process.env.RELAY_WORKSPACE_KEY using
nonEmptyString) and legacyApiKey = first non-empty of
options?.env?.RELAY_API_KEY and process.env.RELAY_API_KEY using nonEmptyString,
then use explicitWorkspaceKey wherever an explicit join is required and only
fall back to legacyApiKey where legacy API-key behavior is intended; apply the
same change to the other resolution sites that use nonEmptyString chains (the
occurrences around the current workspaceKey usage and the similar chains at the
other noted blocks).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 603dfd16-c9bc-433e-a06b-81b00e82f98e
📒 Files selected for processing (8)
.agentworkforce/trajectories/active/traj_b1jrutolckfb/trajectory.jsonCHANGELOG.mdcrates/broker/src/cli/mod.rscrates/broker/src/relaycast/auth.rscrates/broker/src/runtime/init.rscrates/broker/src/spawner.rspackages/harness-driver/src/client.tsrust_out
💤 Files with no reviewable changes (1)
- .agentworkforce/trajectories/active/traj_b1jrutolckfb/trajectory.json
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/harness-driver/src/client.ts">
<violation number="1" location="packages/harness-driver/src/client.ts:490">
P2: Workspace key is passed as a CLI argument, which exposes the secret via process args and startup error logging.</violation>
</file>
<file name="crates/broker/src/cli/mod.rs">
<violation number="1" location="crates/broker/src/cli/mod.rs:280">
P2: Blank `--workspace-key` values are treated as present too early, which can suppress env fallback and silently fall back to workspace creation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Reviewed and fixed PR #1055. Changes made:
Validation run:
No bot review artifacts were present under |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
✅ pr-reviewer applied fixes — committed and pushed Reviewed and fixed PR #1055. Changes made:
Validation run:
No bot review artifacts were present under |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/harness-driver/src/spawn-config.test.ts (1)
6-42: ⚡ Quick winConsider adding test case for
parentEnv.RELAY_API_KEYpreservation.The existing tests cover
options.env.RELAY_API_KEY, but a test verifying thatparentEnv.RELAY_API_KEY(e.g., fromprocess.env) is also preserved without promotion would provide more complete coverage of the legacy-key behavior.🧪 Suggested test case
it('preserves parentEnv.RELAY_API_KEY without promoting to workspace-key argv', () => { const config = buildBrokerSpawnConfig( { cwd: '/tmp/my-project', }, 'br_test', { RELAY_API_KEY: 'rk_live_from_process_env', } ); expect(config.workspaceKey).toBeUndefined(); expect(config.args).not.toContain('--workspace-key'); expect(config.env.RELAY_API_KEY).toBe('rk_live_from_process_env'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/harness-driver/src/spawn-config.test.ts` around lines 6 - 42, Add a unit test to ensure buildBrokerSpawnConfig preserves parentEnv.RELAY_API_KEY without promoting it to a workspace-key argv: call buildBrokerSpawnConfig with no options.env but with a parentEnv object containing RELAY_API_KEY (e.g., 'rk_live_from_process_env'), then assert config.workspaceKey is undefined, config.args does not contain '--workspace-key', and config.env.RELAY_API_KEY equals the parentEnv value; place this alongside the existing tests that reference buildBrokerSpawnConfig so it verifies legacy key preservation from parentEnv/process.env.packages/harness-driver/src/spawn-config.ts (1)
93-98: ⚡ Quick winConsider documenting RELAY_API_KEY exclusion for clarity.
The
workspaceKeyresolution chain intentionally excludesRELAY_API_KEY(from bothoptions?.envandparentEnv) to preserve legacy lenient fallback-to-create behavior, while canonical keys (AGENT_RELAY_WORKSPACE_KEY,RELAY_WORKSPACE_KEY) trigger strict explicit-join mode. This design prevents stale legacy keys from causing hard startup failures. A brief inline comment explaining this exclusion would help future maintainers understand the rationale.📝 Suggested documentation improvement
+ // Resolve workspace key from canonical sources only. RELAY_API_KEY is + // intentionally excluded to preserve legacy lenient behavior (fallback-to-create) + // while canonical keys trigger strict explicit-join mode (fail on reject/rate-limit). const workspaceKey = nonEmptyString(options?.workspaceKey) ?? nonEmptyString(options?.env?.AGENT_RELAY_WORKSPACE_KEY) ??🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/harness-driver/src/spawn-config.ts` around lines 93 - 98, The workspaceKey resolution intentionally omits RELAY_API_KEY (from options?.env and parentEnv) to preserve legacy lenient fallback-to-create behavior; add a concise inline comment next to the workspaceKey assignment (referencing the workspaceKey constant and the resolution chain using AGENT_RELAY_WORKSPACE_KEY and RELAY_WORKSPACE_KEY) explaining that RELAY_API_KEY is deliberately excluded so old/stale legacy keys do not force strict explicit-join mode and cause hard startup failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/harness-driver/src/spawn-config.test.ts`:
- Around line 6-42: Add a unit test to ensure buildBrokerSpawnConfig preserves
parentEnv.RELAY_API_KEY without promoting it to a workspace-key argv: call
buildBrokerSpawnConfig with no options.env but with a parentEnv object
containing RELAY_API_KEY (e.g., 'rk_live_from_process_env'), then assert
config.workspaceKey is undefined, config.args does not contain
'--workspace-key', and config.env.RELAY_API_KEY equals the parentEnv value;
place this alongside the existing tests that reference buildBrokerSpawnConfig so
it verifies legacy key preservation from parentEnv/process.env.
In `@packages/harness-driver/src/spawn-config.ts`:
- Around line 93-98: The workspaceKey resolution intentionally omits
RELAY_API_KEY (from options?.env and parentEnv) to preserve legacy lenient
fallback-to-create behavior; add a concise inline comment next to the
workspaceKey assignment (referencing the workspaceKey constant and the
resolution chain using AGENT_RELAY_WORKSPACE_KEY and RELAY_WORKSPACE_KEY)
explaining that RELAY_API_KEY is deliberately excluded so old/stale legacy keys
do not force strict explicit-join mode and cause hard startup failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9885000b-cfcd-4dc6-af14-98612eef28a0
📒 Files selected for processing (3)
packages/harness-driver/src/client.tspackages/harness-driver/src/spawn-config.test.tspackages/harness-driver/src/spawn-config.ts
2 participants
Summary
Underpins AgentWorkforce/pear#125. Reviewed (no blockers); cargo + harness-driver checks green.
🤖 Generated with Claude Code